Skip to content

Conversation

@stonerl
Copy link

@stonerl stonerl commented Apr 11, 2025

Prettier's formatWithCursor can crash if a node is missing a valid location object.

This patch adds null-safe checks and fallback handling in locStart and locEnd to avoid TypeErrors and ensure graceful degradation.

Prettier's formatWithCursor can crash if a node is missing a valid `location` object.

This patch adds null-safe checks and fallback handling in `locStart` and `locEnd`
to avoid TypeErrors and ensure graceful degradation.

Signed-off-by: Toni Förster <[email protected]>
@kddnewton
Copy link
Member

Can you give me a text XML file where this happens? I have not found an example. If there is a test you could add, that would be great. I don't want to be needlessly defensive here.

@stonerl
Copy link
Author

stonerl commented Apr 12, 2025

@kddnewton no worries. It happens with this test file, with my Prettier extension for Nova.

The error message I get can be found in the issue in the first comment: stonerl/nova-prettier-extension#41 (comment)

@stonerl
Copy link
Author

stonerl commented Apr 12, 2025

It could also be me implementing formatWithCursor incorrect, but I only get this error with the xml-plugin. Everything else works.

@kddnewton
Copy link
Member

I don't think you're doing it wrong, but is it possible you're using an earlier version of the plugin? I cannot get it to crash with the latest version.

@stonerl
Copy link
Author

stonerl commented Apr 12, 2025

I'm using the 3.4.1 that's the latest one AFAIK.

I currently apply this patch when installing all npm packages, and it circumvents the error and allows formatting XML files.

This could be something that happens only in combination with Nova. I don't mind if this patch won't go into upstream, I can keep it as part of my extension.

@stonerl
Copy link
Author

stonerl commented Apr 12, 2025

I think I might know what's going on here. Nova is using Apple's JavaScript Core engine.

I assume you're using node.js for testing? It could be that this is just a platform specific error.

@kddnewton
Copy link
Member

I don't think so, if I run this in bun or safari it passes. I would be shocked if this had anything to do with the runtime as it's a logic error. Are you 100% sure you're on the latest version?

@stonerl
Copy link
Author

stonerl commented Apr 13, 2025

Pretty sure:

nova-prettier-extension/prettier.novaextension on  feature/config-file [!] is 📦 3.5.16 via ⬢ v23.11.0
➜ npm list
[email protected] /Users/toni/Developer/nova-prettier-extension/prettier.novaextension
├── @prettier/[email protected]
├── @prettier/[email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

@stonerl
Copy link
Author

stonerl commented Apr 23, 2025

Just an update. I ran into the same error while implementing pretter-plugin-astro and prettier-plugin-sh and had to fix it the same way.

@kddnewton
Copy link
Member

I think I will leave this as part of your extension, because I cannot figure out how to reproduce this.

@kddnewton kddnewton closed this Jul 8, 2025
@stonerl stonerl deleted the fix-cannot-read-properties branch July 8, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants